-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add ament_cmake_pytest package #116
Conversation
Regarding the test grouping I'm a bit surprised to see (e.g. for rclpy) that the tests in rclpy are not grouped under rclpy but independent: Also I have the same issue as before of not being able to display the dontent of these folders: http://ci.ros2.org/job/ci_linux/3515/testReport/rclpy.src.ros2.rclpy.rclpy.test/. Did anybody succeed to display them ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, I took for granted the logic that was copied for other packages and focused on the new code (has_pytest...).
If I read this correctly, coverage will not be ran by default compared to the pure python packages (which is the right behavior IMO), maybe coverage should be made optional so that we can run it if needed?
# :type QUIET: option | ||
# :param PYTHON_EXECUTABLE: absolute path to the Python interpreter to be used, | ||
# default to the CMake variable with the same name returned by | ||
# FindPythonInterp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be wrapped ? seems more readable if it was on the line above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't fit within 80 characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, in that case, is it possible to wrap the parameter description just above (QUIET) that is much longer than this one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in dcfedfb.
@dirk-thomas can you comment on the questions from #116 (comment) please? I'm not sure if this is expected behavior and how we can mitigate it if it's not. It's pretty inconvenient to not be able to display the test results and need to be resolved otherwise we'll have to revert these PR until we figure it out. |
I would guess that it is related to the custom working directory (https://github.com/ros2/rclpy/blob/309942f48c6543d52ae22b61b98b6bc34759238d/rclpy/CMakeLists.txt#L112). That results in a long relative path for each test. I assume the working directory is set to allow loading the Python extension library (in the build space) from a different location than the Python modules (in the source space). I don't know if there is a way to get rid of the working directory while keeping it working using a different approach.
I would assume that the problem is related to the previous question. Jenkins might not be able to resolve the weird relative path and "hang" itself during that process. |
"-m" "pytest" | ||
"${path}" | ||
# store last failed tests | ||
"-o" "cache_dir=${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_pytest/${testname}/.cache" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Xenial I'm seeing an error with this option when testing rclpy. I installed pytest from apt python3-pytest
.
1: usage: pytest.py [options] [file_or_dir] [file_or_dir] [...]
1: pytest.py: error: unrecognized arguments: -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache
Full output
test 1
Start 1: rclpytests
1: Test command: /usr/bin/python3 "-u" "/workspace/ros2_ws/install_isolated/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/
workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml" "--output-file" "/workspace/ros2_ws/build_isolated/rclpy/
ament_cmake_pytest/rclpytests.txt" "--append-env" "AMENT_PREFIX_PATH=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_index" "LD_LIBRA
RY_PATH=/workspace/ros2_ws/build_isolated/rclpy" "--command" "/usr/bin/python3" "-u" "-m" "pytest" "/workspace/ros2_ws/src/ros2/rclpy/rcl
py/test" "-o" "cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache" "--junit-xml=/workspace/ros2_ws/bu
ild_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml" "--junit-prefix=rclpy"
1: Test timeout computed to be: 90
1: -- run_test.py: extra environment variables to append:
1: - AMENT_PREFIX_PATH+=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_index
1: - LD_LIBRARY_PATH+=/workspace/ros2_ws/build_isolated/rclpy
1: -- run_test.py: invoking following command in '/workspace/ros2_ws/build_isolated/rclpy':
1: - /usr/bin/python3 -u -m pytest /workspace/ros2_ws/src/ros2/rclpy/rclpy/test -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ame
nt_cmake_pytest/rclpytests/.cache --junit-xml=/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml --junit-pre
fix=rclpy
1: usage: pytest.py [options] [file_or_dir] [file_or_dir] [...]
1: pytest.py: error: unrecognized arguments: -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache
1: inifile: None
1: rootdir: /workspace/ros2_ws
1: -- run_test.py: return code 2
1: -- run_test.py: generate result file '/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml' with failed te$
t
1: -- run_test.py: verify result file '/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml'
1/8 Test #1: rclpytests .......................***Failed 0.51 sec
Where is -o cache_dir=
supposed be handled? I haven't found any references to cache_dir
in cpython, unittest, or nose. I see some references to cache_dir
in an ini in pytest here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like cache_dir
was added in version 3.2, but the version in apt is 2.8.7-4
. Installing via pip fixed the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Connect to #115.
See the "nice" test grouping 😉